Skip to content

Comments

stream: accept ArrayBuffer in CompressionStream and DecompressionStream#61913

Open
suuuuuuminnnnnn wants to merge 7 commits intonodejs:mainfrom
suuuuuuminnnnnn:fix/webstreams-arraybuffer-write
Open

stream: accept ArrayBuffer in CompressionStream and DecompressionStream#61913
suuuuuuminnnnnn wants to merge 7 commits intonodejs:mainfrom
suuuuuuminnnnnn:fix/webstreams-arraybuffer-write

Conversation

@suuuuuuminnnnnn
Copy link

@suuuuuuminnnnnn suuuuuuminnnnnn commented Feb 21, 2026

Per the WHATWG Compression Streams spec, CompressionStream and
DecompressionStream must accept BufferSource
(ArrayBuffer | ArrayBufferView) as valid chunk types.

Currently, writing a plain ArrayBuffer to the writable side throws
ERR_INVALID_ARG_TYPE, because the adapter in
newWritableStreamFromStreamWritable passes chunks directly to
stream.write(), which only accepts ArrayBufferView types.

This change normalizes plain ArrayBuffer chunks to a Uint8Array view
before forwarding them to the underlying Node.js stream.

Changes included:

  • normalize ArrayBuffer to Uint8Array in lib/internal/webstreams/adapters.js
  • add a regression test for ArrayBuffer input
  • unskip the relevant WPT compression BufferSource test (if passing)

Fixes: #43433

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/web-standards

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. web streams labels Feb 21, 2026
@suuuuuuminnnnnn suuuuuuminnnnnn force-pushed the fix/webstreams-arraybuffer-write branch from 30ebba1 to 91009bf Compare February 21, 2026 05:04
@codecov
Copy link

codecov bot commented Feb 21, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.75%. Comparing base (ae2ffce) to head (6de7441).
⚠️ Report is 114 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff            @@
##             main   #61913    +/-   ##
========================================
  Coverage   89.75%   89.75%            
========================================
  Files         674      674            
  Lines      204416   204964   +548     
  Branches    39285    39397   +112     
========================================
+ Hits       183472   183966   +494     
- Misses      13227    13269    +42     
- Partials     7717     7729    +12     
Files with missing lines Coverage Δ
lib/internal/webstreams/adapters.js 85.84% <100.00%> (+0.26%) ⬆️

... and 89 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@suuuuuuminnnnnn
Copy link
Author

Thanks @MattiasBuelens and @Renegade334 — applied both suggestions.

  • added the object mode guard before normalizing ArrayBuffer
  • replaced the manual read loop with Array.fromAsync(...)
  • replaced the manual piping/write loop with await cs.readable.pipeTo(ds.writable)

I kept the PR scoped to the ArrayBuffer handling fix + regression coverage.
Appreciate the review.

@MattiasBuelens MattiasBuelens added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 21, 2026
@github-actions github-actions bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Feb 21, 2026
@github-actions

This comment has been minimized.

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 21, 2026
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 21, 2026
@nodejs-github-bot

This comment has been minimized.

@Renegade334 Renegade334 removed the request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. label Feb 21, 2026
@Renegade334 Renegade334 changed the title webstreams: accept ArrayBuffer in WritableStream write sink stream: accept ArrayBuffer in CompressionStream and DecompressionStream Feb 21, 2026
Copy link
Member

@Renegade334 Renegade334 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is pretty much good to go.

One request, though: although testing the compression/decompression stream behaviour is appropriate, the underlying change is to the adapter behaviour, which should also be tested.

Could you please add something like the following to test/parallel/test-whatwg-webstreams-adapters-to-writablestream.js:

{
  const duplex = new PassThrough();
  const writableStream = newWritableStreamFromStreamWritable(duplex);
  const ec = new TextEncoder();
  const arrayBuffer = ec.encode('hello').buffer;
  writableStream
    .getWriter()
    .write(arrayBuffer)
    .then(common.mustCall());

  duplex.on('data', common.mustCall((chunk) => {
    assert(chunk instanceof Buffer);
    assert(chunk.equals(Buffer.from('hello')));
  }));
}

{
  const duplex = new PassThrough({ objectMode: true });
  const writableStream = newWritableStreamFromStreamWritable(duplex);
  const ec = new TextEncoder();
  const arrayBuffer = ec.encode('hello').buffer;
  writableStream
    .getWriter()
    .write(arrayBuffer)
    .then(common.mustCall());

  duplex.on('data', common.mustCall((chunk) => {
    assert(chunk instanceof ArrayBuffer);
    assert.strictEqual(chunk, arrayBuffer);
  }));
}

to test the ArrayBuffer conversion behaviour and honouring objectMode.

@suuuuuuminnnnnn
Copy link
Author

@Renegade334 Thanks for the suggestion — that makes sense.

I’ve added adapter-level tests in test/parallel/test-whatwg-webstreams-adapters-to-writablestream.js covering both behaviors:

  • normalize ArrayBuffer for non-objectMode streams
  • preserve ArrayBuffer for objectMode streams

@MattiasBuelens MattiasBuelens added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 22, 2026
@Renegade334 Renegade334 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Feb 22, 2026
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 22, 2026
@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. needs-ci PRs that need a full CI run. web streams

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Writer cannot cope with ArrayBuffer

6 participants